Reduce memory inefficiencies and leaks

This PR is an effort to reduce various memory inefficiencies and leaks that become mostly apparent if you reuse JavascriptContext objects instead of disposing them after each use. But even if you dispose a context after each use there is a chance you will benefit from these changes, especially if you have a longer running operation that creates a lot of managed objects and calls methods on them. It also remedies the occasional crashes reported e.g. in #90 (see Commit 5).

The PR includes a couple of commits, each of which solves an isolated improvement. I will go over each commit and provide test code, as well as before and after memory usage curves. Overall method used:

1. Write sample code in the Fiddling app to reproduce the inefficiency
2. Run the Fiddling app and observe the memory curve/dump in the diagnostics tools of Visual Studio (I used a Debug build configuration for all of these which I know is not ideal when doing performance optimizations, but IMHO was sufficient to optimize memory use)
3. Then iteratively until the memory curve was flat
   1. Try to fix the inefficiency
   2. Run the Fiddling app again to see improvements/degradations

Test code

The following test code was used to discover and solve all inefficiencies. The overall structure was the same for all tests, only a couple of lines were added after each commit. Each test case is marked with a comment that will be referenced in the explainer for the corresponding commit. Also, each breakpoint location I used is marked with a comment.

public class Product
{
    public Product(decimal price)
    {
        Price = price;
    }
    public decimal Price { get; set; }
    public void DoSomething() { }
    public void DoSomethingElse() { }
    public IEnumerable<decimal> GetTaxes() => new List<decimal> { 0.01m, 0.02m };
    public decimal GetSalesTax(JavascriptFunction callback) => Convert.ToDecimal(callback.Call(Price));
    public override string ToString() => Price.ToString();
}

// ...

static void Main(string[] args)
{
    using (JavascriptContext context = new JavascriptContext())
    {
        try
        {
            // breakpoint here
            context.SetConstructor<Product>("Product", (Func<decimal, Product>) (price => new Product(price)));
            context.SetParameter("globalProduct", new Product(2));
            var result = context.Run($@"
{{
    const importantProduct = new Product(3);
    let sum = 0;
    for (let i = 0; i < 200_000; i++) {{

        // Commit 1 - creating managed objects from JS
        const product = new Product(Math.random());

        // Commit 2 - calling methods on managed objects
        product.DoSomething();
        product.DoSomething();
        product.DoSomethingElse();

        // Commits 3 and 4 - using iterators
        for (const tax of product.GetTaxes())
        {{
            sum += tax;
        }}

        // Commit 5 - using JS callbacks in managed code without disposing them explicitly
        sum += product.GetSalesTax(p => p * 0.19);
        // End of scenarios

        sum += product.Price;
    }}
    [sum, importantProduct.Price, globalProduct.Price].toString();
}}
");
            Console.WriteLine(result);
            Console.WriteLine(context.GetParameter("globalProduct"));
            // breakpoint here - pre dispose of the context
        }
        catch (Exception ex)
        {
            string s = (string)ex.Data["V8StackTrace"];
            Console.WriteLine(s);
        }
    }
    GC.Collect();
    GC.WaitForPendingFinalizers();
    GC.Collect();
    // breakpoint here - after dispose of the context (the garbage collection was only triggered to compare the object count in the memory dump)
}

Commit 1 - Set up a callback function that runs when a JS object becomes weak

In this scenario we create a lot of managed Product objects from JS code.

Since each object is scoped only to one iteration of the for-loop the .NET garbage collector (GC) should be able to collect these objects while the loop is running, resulting in a flat memory curve. However, each object is held with a strong GC handle here

Memory curve before (1,000,000 iterations; 36.28s runtime in the diagnostic session)

To solve this problem we keep track of a v8::Persistent for the object and set a callback function on it that runs when V8 deems an object as weak (i.e. the object only has weak references left in the V8 context, if any at all) using the SetWeak method. For this we use a new helper method Wrap on JavascriptExternal that sets the internal field of the v8::Local wrapper object, initializes the v8::Persistent for it, and sets the callback.

When the callback runs we a) remove the WrappedExternal from the tracked externals in JavascriptContext and b) free the underlying pointer which leads to the execution of the JavascriptExternal destructor and therefore the freeing of the GC handle. The GC is now able to collect the managed object.

Memory curve after (1,000,000 iterations; 36s runtime in the diagnostic session)

Commit 2 - Cache WrappedMethod at context level

In this scenario we additionally call two dictinct methods on each created object. One of those methods is called twice.

Each JavascriptExternal holds a cache of WrappedMethod objects with each method that was called on the underlying managed object here

Memory curve before (200,000 iterations; 33.93s runtime in the diagnostic session)

We do not need to cache in this way however, because both a v8::FunctionTemplate and the concrete v8::Function live at the v8::Context level and are additionally totally independent from the concrete object on which we call the method. Think V8 calling a method like functionInstance.call(objectInstance, <arguments>). Therefore we can save a ton of allocations by caching those functions by assembly qualified name and method name at the JavascriptContext level instead, and reusing them for different object instances.

The only thing we must provide when creating the v8::FunctionTemplate is the method name so that the JavascriptInterop::Invoker method can find the method by reflection here

This change had the overall biggest impact.

Memory curve after (200,000 iterations; 18.46s runtime in the diagnostic session)

Commits 3 and 4 - Reduce wrapping in iterator callbacks and use method cache

In this scenario we use a for-of loop to iterate through a collection on each object. The collection is returned by yet another method we call.

These commits align the creation of iterator callbacks with the technique used in Commit 2. They don't improve memory usage measurably in the above test, but simplify and unify code structure.

Commit 5 - Prefer a weak reference to the JavascriptContext in JavascriptFunction

In this scenario we additionally call a method that has a JavascriptFunction callback as parameter that is not disposed explicitly.

Instead of holding a list of JavascriptFunction handles in the JavascriptContext here

For reference: The destructor ~JavascriptFunction is called when using the Dispose method explicitly from managed code. The finalizer !JavascriptFunction (that caused the memory access violations) is called when the GC collects the object which could happen after the JavascriptContext was destroyed.

It also simplifies code in JavascriptContext which now does not need to know about JavascriptFunction handles that the GC takes care of anyway. Instead we just check if the JavascriptContext the function was created in is still "alive" and only call the Reset method on the v8::Persistent in that case (see ~JavascriptFunction). If the V8 context has already been disposed the call to Reset is irrelevant because the underlying memory has already been freed.

Memory curve before (200,000 iterations; 37.32s runtime in the diagnostic session)

Memory curve after (200,000 iterations; 33.54s runtime in the diagnostic session)

Conclusion

When not using any JS callbacks in managed code (i.e. JavascriptFunction instances), we have now a completely flat curve when creating thousands of managed objects from JS and calling methods on them. We still leak something when callbacks are involved and the memory use is therefore still increasing in the scenario of Commit 5. But compared to what we had before this is a great improvement. I haven't had the chance yet to analyze the remaining leak and would do that if I find time at a later date in another PR. For now I'm quite happy with the results.

Running the complete test code without the callback scenario on the master branch we get the following curve (200,000 iterations; 46.17s runtime in the diagnostic session)

Running the complete test code without the callback scenario with this PR we get the following curve (200,000 iterations; 26.17s runtime in the diagnostic session)